Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1036 +/- ##
==========================================
- Coverage 74.05% 73.63% -0.42%
==========================================
Files 39 44 +5
Lines 6495 6927 +432
Branches 1122 1173 +51
==========================================
+ Hits 4810 5101 +291
- Misses 1230 1342 +112
- Partials 455 484 +29
🚀 New features to boost your workflow:
|
|
|
||
|
|
||
| # --- Registry --- | ||
|
|
There was a problem hiding this comment.
like the idea with the metric registry puts structure into complicated input types. Should consider adopting something like this more generally. Also do you know how well this works with spatialdata images and if they use something similar?
There was a problem hiding this comment.
like the idea with the metric registry puts structure into complicated input types.
agree!
Also do you know how well this works with spatialdata images and if they use something similar?
Wdym?
There was a problem hiding this comment.
Wdym?
like in general I wonder if there is something already similar to this in spatialdata codebase. Not the registry itself but the InputKind.
There was a problem hiding this comment.
Ahh, not that I'm aware of 🤔 We're channel agnostic. In sdata-plot I try to infer this from the channel names but formally it doesn't exist
selmanozleyen
left a comment
selmanozleyen
left a comment
There was a problem hiding this comment.
Things to do:
- Sync with main (some files are old)
- Vectorize for loops I mentioned
- Use scikit-image when we can
timtreis
force-pushed
the
bugfix/issue1034-function-to-qc-he-images
branch
from
e4890cc to
5952a86
Compare
Replaces the earlier qc_sharpness prototype with a general-purpose qc_image function that computes tile-based QC metrics on spatial images. Compute (sq.experimental.im.qc_image): - Tile-based metrics: sharpness (tenengrad, var_of_laplacian), intensity (brightness, entropy), staining (hematoxylin/eosin via HED deconvolution), and artifact detection (fold fraction, tissue fraction) - QCMetric enum and registry mapping each metric to its input kind and callable - Percentile-rank unfocus scoring within tissue tiles for outlier detection - Preview overlay showing flagged tiles on the image - Shared utilities in _utils.py: vectorized TileGrid (numpy + shapely.box), mask helpers, and shapes persistence (also used by make_tiles) Plot (sq.experimental.pl.qc_image): - Multi-panel summary: spatial view, KDE distribution (tissue vs background), and descriptive statistics per metric Metrics use scikit-image filters (sobel_h/v, laplace) instead of hand-rolled convolutions, and thread-safe HED caching avoids redundant deconvolution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
timtreis
force-pushed
the
bugfix/issue1034-function-to-qc-he-images
branch
from
5952a86 to
a5e4dfd
Compare
|
|
||
|
|
||
| # --- Intensity metrics (grayscale input) --- | ||
|
|
There was a problem hiding this comment.
Maybe you aren't done yet but while I have it in mind I want to say I'd put intensity metrics in another file like done in sharpness metrics and have the registry in shorter file.
Also functions called outside a module should not start with _. squidpy.experimental.im._sharpness_metrics
is already private so _tenengrad_mean can be tenengrad_mean. Thats why in your IDE it might show _tenengrad_mean as unused function because the assumption is _tenengrad_mean is only meant to be used in the function it is defined.
…ficiency - Rename _ensure_tissue_mask → _resolve_tissue_mask (clearer intent) - Split intensity/staining/artifact metrics into _intensity_metrics.py, keeping _qc_metrics.py as a lean registry + enums module - Remove redundant tissue_similarity/background_similarity columns - Simplify _detect_tissue_from_mask → _classify_tiles_by_tissue (accept TileGrid, return 2 arrays instead of 4, reuse pre-binarized mask) - Cache FFT frequency grid via @lru_cache (avoid recomputing per tile) - Pre-compute grayscale luminance weights as module-level constant - Fix O(n) var_names lookup in plotting with dict - Reduce allocations in _entropy - Fix stats text indentation in plot panel - Revert stale .pre-commit-config.yaml pins to match main - Document _fold_fraction HSV thresholds as H&E-tuned Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- test_plot_calc_qc_image_not_hne: use metrics=None so is_hne=False actually exercises different default metrics (was identical to hne test) - test_qc_image_rgb_metric: add explicit is_hne=True for clarity - Add test_qc_image_outlier_detection_with_tissue: verify outlier and tissue columns are populated correctly - Add test_qc_image_outlier_detection_without_tissue: verify detect_tissue=False path (no tissue columns, no NaN scores) - Add test_qc_image_compute_only: verify minimal compute-only path Note: QCImage_calc_qc_image_not_hne.png reference image needs regeneration since the test now produces a different plot. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@timtreis could you resolve the conflicts please its hard to review like this |
…tion-to-qc-he-images # Conflicts: # .pre-commit-config.yaml # src/squidpy/experimental/im/_make_tiles.py
| from squidpy.experimental.im._intensity_metrics import ( | ||
| _brightness_mean, | ||
| _brightness_std, | ||
| _entropy, |
There was a problem hiding this comment.
Again, if _entropy is needed to import then no need to make it file/module private. _intensity_metrics is already private
| return np.array([[float(np.var(b))]], dtype=np.float32) | ||
|
|
||
|
|
||
| @lru_cache(maxsize=4) |
There was a problem hiding this comment.
is there a reason why this is 4? at least can we have a comment here to justify the heuristic?
2 participants
IMPORTANT: Please search among the Pull requests before creating one.
Description
How has this been tested?
Closes